HYPERFLEET-975 - ci: add migration immutability check by kuudori · Pull Request #114 · openshift-hyperfleet/hyperfleet-api

kuudori · 2026-04-27T21:05:56Z

Add verify-migrations.sh that detects modifications to existing migration files by diffing against the PR base commit.

New hack/verify-migrations.sh: fails if existing migration files under pkg/db/migrations/ are modified, renamed, or deleted
Uses PULL_BASE_SHA in Prow, falls back to merge-base locally
Add verify-migrations Makefile target
Wire verify-migrations into the test target

Summary

HYPERFLEET-975

Test Plan

Unit tests added/updated
make test-all passes
make lint passes
Helm chart changes validated with make test-helm (if applicable)
Deployed to a development cluster and verified (if Helm/config changes)
E2E tests passed (if cross-component or major changes)

Summary by CodeRabbit

Tests
- Added an automated migration immutability check that runs before unit tests, preventing accidental edits to existing migrations.
- When violations are found the verification fails and reports offending files with guidance to add an additive migration.
Chores
- Integrated migration verification into CI so migration compliance is enforced as part of the standard verification pipeline.

coderabbitai · 2026-04-27T21:06:10Z

Walkthrough

Adds a migration immutability check: a new script hack/verify-migrations.sh determines a git base (using PULL_BASE_SHA or git merge-base HEAD origin/main), diffs Go files in pkg/db/migrations (excluding migration_structs.go) for modifications/renames/deletions, and fails if any are found. A .PHONY: verify-migrations Makefile target runs that script, and verify-migrations was added as a prerequisite to the verify-all target so the check runs when verify-all is invoked.

Sequence Diagram(s)

sequenceDiagram
  participant CI
  participant Makefile
  participant Script as hack/verify-migrations.sh
  participant Git
  participant TestRunner

  CI->>Makefile: invoke "verify-all"
  Makefile->>Script: run hack/verify-migrations.sh
  Script->>Git: determine base (PULL_BASE_SHA or git merge-base)
  Script->>Git: diff pkg/db/migrations/*.go (exclude migration_structs.go)
  Git-->>Script: list of changed migration files (or none)
  alt violations found
    Script-->>Makefile: exit 1 + failure message
    Makefile-->>CI: abort verify-all
  else no violations
    Script-->>Makefile: success
    Makefile->>TestRunner: continue with remaining verify-all steps (lint, test)
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding a migration immutability check to the CI pipeline via a new verification script and Makefile target.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@hack/verify-migrations.sh`:
- Around line 12-15: The current immutability check assigns VIOLATIONS by
diffing all "${MIGRATION_DIR}/*.go", which incorrectly freezes
migration_structs.go (which must be edited to append new entries to
MigrationList); update the VIOLATIONS computation (the assignment referencing
VIOLATIONS and MIGRATION_DIR) to only include timestamped migration files (e.g.,
match the filename pattern used for individual migrations) or explicitly exclude
migration_structs.go so that additive changes can update MigrationList without
failing the check.

In `@Makefile`:
- Line 202: The ci-test-unit Makefile target is missing the verify-migrations
dependency, so running CI with ci-test-unit bypasses hack/verify-migrations.sh;
update the ci-test-unit target to include verify-migrations (same way the test
target does) so that ci-test-unit depends on verify-migrations and thus runs
hack/verify-migrations.sh before executing tests.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: f9818caa-70b5-41d1-9e3f-0f997e09a0ce

📥 Commits

Reviewing files that changed from the base of the PR and between 12333a0 and 1e1270b.

📒 Files selected for processing (2)

Makefile
hack/verify-migrations.sh

kuudori · 2026-04-27T21:41:49Z

/retest

coderabbitai

🧹 Nitpick comments (1)

hack/verify-migrations.sh (1)

10-10: Add explicit error handling for the base resolution command.

Line 10 can fail abruptly under set -e when the git merge-base command fails—whether due to a missing origin/main or no common ancestry. Without an explicit guard, the script terminates without guidance. Add error handling with a clear message:

Suggested change

-BASE="${PULL_BASE_SHA:-$(git merge-base HEAD origin/main)}"
+if [[ -n "${PULL_BASE_SHA:-}" ]]; then
+    BASE="${PULL_BASE_SHA}"
+else
+    if ! BASE="$(git merge-base HEAD origin/main 2>/dev/null)"; then
+        echo "FAIL: unable to resolve base commit. Ensure origin/main exists and HEAD has common history, or set PULL_BASE_SHA." >&2
+        exit 1
+    fi
+fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@hack/verify-migrations.sh` at line 10, The BASE assignment using
PULL_BASE_SHA and git merge-base can fail under set -e; update the BASE
resolution in hack/verify-migrations.sh to run the git merge-base command with
explicit error handling: run the command, capture its output into BASE (or a
temp var), check if it succeeded and that BASE is non-empty, and if it failed
print a clear error mentioning git merge-base and origin/main (or missing
PULL_BASE_SHA) and exit non-zero; ensure references to BASE and the git
merge-base invocation (the current BASE="${PULL_BASE_SHA:-$(git merge-base HEAD
origin/main)}" logic) are updated so failures produce a helpful message rather
than an abrupt script termination.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@hack/verify-migrations.sh`:
- Line 10: The BASE assignment using PULL_BASE_SHA and git merge-base can fail
under set -e; update the BASE resolution in hack/verify-migrations.sh to run the
git merge-base command with explicit error handling: run the command, capture
its output into BASE (or a temp var), check if it succeeded and that BASE is
non-empty, and if it failed print a clear error mentioning git merge-base and
origin/main (or missing PULL_BASE_SHA) and exit non-zero; ensure references to
BASE and the git merge-base invocation (the current BASE="${PULL_BASE_SHA:-$(git
merge-base HEAD origin/main)}" logic) are updated so failures produce a helpful
message rather than an abrupt script termination.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 5fc01df9-90f8-46ea-85e6-86345919ecb3

📥 Commits

Reviewing files that changed from the base of the PR and between d7a50d1 and 57299dd.

📒 Files selected for processing (2)

Makefile
hack/verify-migrations.sh

🚧 Files skipped from review as they are similar to previous changes (1)

Makefile

rafabene · 2026-04-29T16:21:24Z

/lgtm

rafabene · 2026-04-29T18:49:41Z

/lgtm

rafabene · 2026-04-29T18:50:41Z

/approve

openshift-ci · 2026-04-29T18:57:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rafabene

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [rafabene]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci Bot requested review from Mischulee and jsell-rh April 27, 2026 21:06

coderabbitai Bot reviewed Apr 27, 2026

View reviewed changes

Comment thread hack/verify-migrations.sh Outdated

Comment thread Makefile Outdated

kuudori force-pushed the HYPERFLEET-975 branch from 1e1270b to d7a50d1 Compare April 27, 2026 21:17

HYPERFLEET-975 - ci: add migration immutability check

57299dd

kuudori force-pushed the HYPERFLEET-975 branch from d7a50d1 to 57299dd Compare April 27, 2026 21:49

coderabbitai Bot reviewed Apr 27, 2026

View reviewed changes

rafabene reviewed Apr 29, 2026

View reviewed changes

Comment thread hack/verify-migrations.sh

rafabene reviewed Apr 29, 2026

View reviewed changes

Comment thread hack/verify-migrations.sh

openshift-ci Bot assigned rafabene Apr 29, 2026

openshift-ci Bot added lgtm approved labels Apr 29, 2026

kuudori mentioned this pull request Apr 29, 2026

HYPERFLEET-975 - ci: add verify-migrations presubmit job for hyperfleet-api openshift/release#78434

Open

kuudori requested a review from rafabene April 29, 2026 18:45

rafabene approved these changes Apr 29, 2026

View reviewed changes

rafabene merged commit 3622815 into openshift-hyperfleet:main Apr 29, 2026
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HYPERFLEET-975 - ci: add migration immutability check#114

HYPERFLEET-975 - ci: add migration immutability check#114
rafabene merged 1 commit intoopenshift-hyperfleet:mainfrom
kuudori:HYPERFLEET-975

kuudori commented Apr 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 27, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

kuudori commented Apr 27, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

rafabene commented Apr 29, 2026

Uh oh!

rafabene commented Apr 29, 2026

Uh oh!

rafabene commented Apr 29, 2026

Uh oh!

openshift-ci Bot commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kuudori commented Apr 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Sequence Diagram(s)

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kuudori commented Apr 27, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rafabene commented Apr 29, 2026

Uh oh!

rafabene commented Apr 29, 2026

Uh oh!

rafabene commented Apr 29, 2026

Uh oh!

openshift-ci Bot commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kuudori commented Apr 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 27, 2026 •

edited

Loading